Closed Bug 1958839 Opened 2 months ago Closed 27 days ago

Remove support for 'discard' element

Categories

(Core :: SVG, task)

task

Tracking

()

RESOLVED FIXED
140 Branch
Tracking Status
firefox139 --- fixed
firefox140 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

We recently landed a patch to add experimental support for the SVG <discard> element, partly on the hypothesis that doing so might prompt Chromium engineers to add back their historical implementation of that element (if their removal was largely due to lack-of-support-in-other-browsers and if adding it back was not to much trouble).

However, since then:
(1) we ran across a bug in our experimental implementation; this bug indicated that there's some added complexity in making our implementation robust, and it prompted us to disable the feature (in bug 1954608).
(2) we ran across https://github.com/w3c/svgwg/issues/717#issue-470289163 where Chromium implementors requested to remove the feature from the spec when they un-shipped the feature -- they expressed additional reasons that they opted to remove the feature beyond lack-of-support-in-other-browsers, and they seem disinclined to add the feature back, given that the cost/benefit is not particularly favorable. (They note that the complexity & perf cost was nontrivial, and usage of the feature was ~zero, and it doesn't add much in the way of utility aside from the potential for SVG authors to reduce memory usage in long-running unscripted SVG animations.)
(3) @zcorpan closed the loop on the issue referenced in (2), removing the feature from the SVG spec.

Given the above (particularly (2)), it appears that the the benefits of this feature are not worth the maintenance burden & increased attack-surface cost of having this feature. Let's remove our implementation.

Added dev-doc-needed. MDN will need some cleanup if this happens.

Keywords: dev-doc-needed
Blocks: 1959097
See Also: → 1964009

See reasoning in https://bugzilla.mozilla.org/show_bug.cgi?id=1958839#c0

This feature was already off by default. This patch is a squashed backout of
the following 6 changesets, listed in reverse chronological order:

https://hg.mozilla.org/mozilla-central/rev/efb626444437
https://hg.mozilla.org/mozilla-central/rev/ae8af0edf737
https://hg.mozilla.org/mozilla-central/rev/dd3fac9da2c1
https://hg.mozilla.org/mozilla-central/rev/d5e958d3caae
https://hg.mozilla.org/mozilla-central/rev/acdd71cd851b
https://hg.mozilla.org/mozilla-central/rev/86f62a648ef0

Fortunately none of the backouts needed manual merge-conflict-resolution,
so this is an entirely mechanical backout patch.

Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Duplicate of this bug: 1957980

(Once we've landed on central, we should uplift to beta139 as well, since our preffed-off <discard> implementation inadvertently exposed a stub version of the SVGDiscardElement interface, which is a bit of a wart & is potentially confusing, per bug 1964009 and bug 1957980 and bug 1959097.)

Blocks: 1964242
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/98146d3f4937 Remove implementation of SVG <discard> element. r=firefox-svg-reviewers,layout-reviewers,webidl,saschanaz,longsonr,emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/52332 for changes under testing/web-platform/tests
Duplicate of this bug: 1964009
Status: ASSIGNED → RESOLVED
Closed: 27 days ago
Resolution: --- → FIXED
Target Milestone: --- → 140 Branch
Upstream PR merged by moz-wptsync-bot

We recently added experimental support for this element only recently, and it
was never enabled by default on release.

Our implementation has been preffed-off on all branches for a little while via
bug 1954608; and this patch removes the implementation entirely, for reasons
discussed in https://bugzilla.mozilla.org/show_bug.cgi?id=1958839#c0

This patch is a squashed backout of the following 6 changesets, listed in
reverse chronological order:

https://hg.mozilla.org/mozilla-central/rev/efb626444437
https://hg.mozilla.org/mozilla-central/rev/ae8af0edf737
https://hg.mozilla.org/mozilla-central/rev/dd3fac9da2c1
https://hg.mozilla.org/mozilla-central/rev/d5e958d3caae
https://hg.mozilla.org/mozilla-central/rev/acdd71cd851b
https://hg.mozilla.org/mozilla-central/rev/86f62a648ef0

Fortunately none of the backouts needed manual merge-conflict-resolution,
so this is an entirely mechanical backout patch.

Original Revision: https://phabricator.services.mozilla.com/D247668

Attachment #9486159 - Flags: approval-mozilla-beta?

firefox-beta Uplift Approval Request

  • User impact if declined: Low; we just expose a "SVGDiscardElement" interface for a feature that's actually turned off (see bug 1964009) and that we don't intend to ship. It's conceivable that sites might get confused by the presence of that interface and might think it works or use it in a weird way.
  • Code covered by automated testing: yes
  • Fix verified in Nightly: yes
  • Needs manual QE test: no
  • Steps to reproduce for manual QE testing: N/A
  • Risk associated with taking this patch: Minimal; this is just a rollout of backout patches for a feature that we never shipped and that we've opted to remove
  • Explanation of risk level: Minimal
  • String changes made/needed: None
  • Is Android affected?: yes
Attachment #9486159 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9486159 - Flags: approval-mozilla-beta+ → approval-mozilla-beta-
Attachment #9486159 - Flags: approval-mozilla-beta- → approval-mozilla-beta+

FF139 Docs work for this done/tracked in https://github.com/mdn/content/issues/39618

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: